Skip to content

add nicmacaddresslist parameter to importUnmanagedInstance and importVM APIs#13414

Open
prashanthr2 wants to merge 10 commits into
apache:mainfrom
prashanthr2:feature/importvm-mac-address-support
Open

add nicmacaddresslist parameter to importUnmanagedInstance and importVM APIs#13414
prashanthr2 wants to merge 10 commits into
apache:mainfrom
prashanthr2:feature/importvm-mac-address-support

Conversation

@prashanthr2

Copy link
Copy Markdown
Contributor

Description

Adds support for specifying per-NIC MAC addresses when importing a VM into CloudStack via importUnmanagedInstance and importVM APIs.

Problem

Both APIs always used the MAC address reported by the hypervisor. The only MAC control was the forced boolean — which either rejected a conflict or silently generated a new MAC. There was no way to supply a specific MAC at import time, which is needed for migration scenarios requiring MAC preservation.

Solution

New MAP parameter nicmacaddresslist following the same pattern as nicnetworklist and nicipaddresslist:

```
nicmacaddresslist[0].nic=nic1&nicmacaddresslist[0].mac=aa:bb:cc:dd:ee:ff
nicmacaddresslist[1].nic=nic2&nicmacaddresslist[1].mac=aa:bb:cc:dd:ee:02
```

NICs not listed keep their hypervisor-reported MAC — fully backward compatible.

Changes

  • ApiConstants: add NIC_MAC_ADDRESS_LIST = "nicmacaddresslist"
  • VmDetailConstants: add NIC_MAC_ADDRESS = "macAddress"
  • ImportUnmanagedInstanceCmd: new @Parameter nicmacaddresslist with getNicMacAddressList()isValidMac + isUnicastMac + standardizeMacAddress validation
  • UnmanagedVMsManagerImpl:
    • mergeNicMacAddresses(): merges caller MACs into the IpAddresses map
    • baseImportInstance() + importVm(): call mergeNicMacAddresses()
    • importNic(): prefers caller-supplied MAC over hypervisor-reported MAC
    • importNic(): privateprotected for testability

Tests

  • ImportUnmanagedInstanceCmdTest (new): 9 tests — MAC format, unicast, empty/null validation
  • UnmanagedVMsManagerImplTest: 8 new tests — mergeNicMacAddresses() edge cases + importNic() MAC selection

Prashanth Reddy added 10 commits June 11, 2026 20:32
…importVM APIs

Allow callers to specify per-NIC MAC addresses when importing a VM into
CloudStack management, overriding the MAC address reported by the hypervisor.

Problem
-------
importUnmanagedInstance and importVM always used the MAC address reported by
the hypervisor. There was no way to supply a specific MAC at import time.
The only control was 'forced': reject conflicting MAC or silently replace it.

Solution
--------
Add new MAP parameter 'nicmacaddresslist' to ImportUnmanagedInstanceCmd
(inherited by ImportVmCmd), following the pattern of 'nicnetworklist' and
'nicipaddresslist'.

  nicmacaddresslist[0].nic=nic1&nicmacaddresslist[0].mac=aa:bb:cc:dd:ee:ff
  nicmacaddresslist[1].nic=nic2&nicmacaddresslist[1].mac=aa:bb:cc:dd:ee:02

NICs not listed keep their hypervisor-reported MAC (backward compatible).

Changes
-------
- ApiConstants: add NIC_MAC_ADDRESS_LIST = "nicmacaddresslist"
- VmDetailConstants: add NIC_MAC_ADDRESS = "mac" (map entry key)
- ImportUnmanagedInstanceCmd: add @parameter nicmacaddresslist with
  getNicMacAddressList() validation (isValidMac + isUnicastMac +
  standardizeMacAddress), matching BaseDeployVMCmd / AddNicToVMCmd
- UnmanagedVMsManagerImpl:
  * mergeNicMacAddresses() merges caller MACs into the IpAddresses map
  * baseImportInstance() and importVm() call mergeNicMacAddresses()
  * importNic() prefers ipAddresses.getMacAddress() (caller) over
    nic.getMacAddress() (hypervisor) when non-empty
  * importNic() changed private -> protected for testability

Tests
-----
- ImportUnmanagedInstanceCmdTest: MAC format/unicast/empty validation
- UnmanagedVMsManagerImplTest: mergeNicMacAddresses() edge cases +
  importNic() MAC selection (caller preferred, fallback to hypervisor)
…xception

Remove test-specific stubs for dataCenterDao and networkOrchestrationService.importNic
that duplicate the setUp() stubs. With Mockito 5 strict stubs, test-method stubs that
don't get invoked (because setUp's broader nullable() stubs match first) trigger
UnnecessaryStubbingException. Rely on setUp's stubs and only verify the MAC argument.
Normalize all changed files to LF line endings, strip trailing whitespace,
and ensure a single newline at end of file to pass pre-commit hooks:
end-of-file-fixer, mixed-line-ending, trailing-whitespace.
@codecov

codecov Bot commented Jun 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 18.77%. Comparing base (b0601e5) to head (44a8853).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##               main   #13414    +/-   ##
==========================================
  Coverage     18.76%   18.77%            
- Complexity    17971    17999    +28     
==========================================
  Files          6160     6163     +3     
  Lines        552571   552816   +245     
  Branches      67346    67374    +28     
==========================================
+ Hits         103694   103788    +94     
- Misses       437469   437624   +155     
+ Partials      11408    11404     -4     
Flag Coverage Δ
uitests 3.53% <ø> (ø)
unittests 19.96% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant